-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Add autoloading tutorial #3037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add autoloading tutorial #3037
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/tutorials/3037
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit d45477c with merge base d862a95 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Please make sure to review the Tutorial Submission Policy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing a tutorial on this. I think it is indeed a good topic to cover.
I think it would be good to take a step back and clarify who the audience is for this and focus the content on providing the information that audience is looking for.
In particular, my expectation is that the audience are the third party extension writers who should use this feature to improve seamless integration within PyTorch. In that case,
I will keep the intro at the top but make the "How to apply this to out-of-tree extensions the main section after that.
The How it works is covered in the RFC IIRC, maybe linking to it would be enough? I do agree that pointing out the env variable to disable this feature is a good thing to mention here though.
For the examples section, that makes sense to have this at the end as "example projects using this feature as well" but I would limit it to code pointers into how they did the integration on their github repos code. I don't think the code samples add much?
@svekars @albanD Thanks so much for your comments!
Oh I'm so sorry, I will make sure to review this policy and adhere to it.
Thanks for your professional review! I totally agree with your suggestion. I'm making changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-shuffle looks good. Thanks for the quick update.
Only the TODO in the examples section but the rest sounds good to me at a high level. Will let @svekars review in case there is any finer grain issue needed.
@jgong5 Please review the HPU example section, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diagram seems missing a pointer from import torch
to the entry points to load - the loading of entry points is triggered by import torch
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I will make some changes to this diagram
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jgong5 updated, please have a look
Co-authored-by: Jiong Gong <[email protected]>
Co-authored-by: Jiong Gong <[email protected]>
`habana_frameworks.torch`_ is a Python package that enables users to run | ||
PyTorch programs on Intel Gaudi via the PyTorch ``HPU`` device key. | ||
``import habana_frameworks.torch`` is no longer necessary after this mechanism | ||
is applied. | ||
|
||
.. _habana_frameworks.torch: https://docs.habana.ai/en/latest/PyTorch/Getting_Started_with_PyTorch_and_Gaudi/Getting_Started_with_PyTorch.html | ||
|
||
.. code-block:: diff | ||
|
||
import torch | ||
import torchvision.models as models | ||
- import habana_frameworks.torch # <-- extra import | ||
model = models.resnet50().eval().to("hpu") | ||
input = torch.rand(128, 3, 224, 224).to("hpu") | ||
output = model(input) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jgong5 should the implementation example of this mechanism be added here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please leave it here for now. @bsochack is the habana bridge ready for autoloading?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is implemented and verified. Not yet released.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bsochack we need an implementation example here, can you provide one? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any suggestion how it should be provided?
The only challenge that we had is the circular dependencies. We solved it in the way below. Other than that, it worked out of box.
habana_frameworks/init.py:
import os
is_loaded = False # A member variable of habana_frameworks module to track if our module has been imported
def __autoload():
# This is an entrypoint for pytorch autoload mechanism
# If the following confition is true, that means our backend has already been loaded, either explicitly
# or by the autoload mechanism and importing it again should be skipped to avoid circular imports
global is_loaded
if is_loaded:
return
import habana_frameworks.torch
habana_frameworks/torch/init.py:
import os
# This is to prevent torch autoload mechanism from causing circular imports
import habana_frameworks
habana_frameworks.is_loaded = True
and one addition in setup.py:
entry_points={
"torch.backends": [
"device_backend = habana_frameworks:__autoload",
],
},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bsochack thanks so much for your quick reply!
The circular dependency issue is worth noting, I will state this in this doc.
I noticed habana_frameworks is not an open-source project, so are there any public links about the autoloading mechanism? It's OK if not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, habana_frameworks is not open source and there is no official release with the above change.
Btw, it requires PT 2.5 (not yet released) so it is hard to provide a real example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bsochack OK it's alright
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bsochack updated, please take a look at the HPU section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
Co-authored-by: Svetlana Karslioglu <[email protected]>
@svekars Thanks so much for your professional and patient review! I made some changes which can be found at: https://github.com/pytorch/tutorials/pull/3037/files/4d44a78d348b918a5f6a3ff60f804b330e983546..e425fe90aeca2b734c390ddd4e0fadc1f0f715fd |
Hi @svekars, just a gentle reminder about this PR. I'm hoping your review and thanks for your time! |
It's LGTM. One question - it is a prototype feature, correct? Can we put it under the prototype_source, then? |
@svekars Yes, I'm doing this. But will it always be under the prototype_source, or will it be moved back after pytorch2.5 is released? |
@svekars The move is done, could you please review it again? |
@shink my understanding is that this feature is released as a prototype in 2.5 so it should remain there until Beta status is reached in one of the future releases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@svekars thanks for your approval and can this pr be merged? :D |
Fixes #3039
This PR adds a tutorial about the out-of-tree extension autoloading mechanism introduced in pytorch/pytorch#122468 and implemented in pytorch/pytorch#127074
cc: @jgong5 @Yikun @hipudding @FFFrog @albanD